Skip to content

feat: add L2PS messaging client and protocol types#82

Merged
tcsenpai merged 6 commits intomainfrom
feat/l2ps-messaging-types
Mar 26, 2026
Merged

feat: add L2PS messaging client and protocol types#82
tcsenpai merged 6 commits intomainfrom
feat/l2ps-messaging-types

Conversation

@Shitikyan
Copy link
Copy Markdown
Contributor

@Shitikyan Shitikyan commented Mar 18, 2026

  • L2PSMessagingPeer: WebSocket client with auto-reconnect, event handlers, request-response pattern
  • l2ps_types: full protocol types (client/server messages, encryption, storage, errors)
  • L2PSInstantMessagingPayload: transaction payload type for L2PS-backed messaging
  • Updated Transaction union type and exports

Summary by CodeRabbit

  • New Features

    • L2PS-backed instant messaging: connect/disconnect, registration, encrypted send with sent/queued acknowledgements, history (paginated), peer discovery, public-key lookup, event callbacks, offline queuing and automatic reconnection.
    • New typed protocol and payload support enabling L2PS message frames and a new instant-messaging transaction payload.
  • Documentation

    • Added guidance sections with mental frameworks for project/task management.
  • Chores

    • Updated project config and ignore rules.

- L2PSMessagingPeer: WebSocket client with auto-reconnect, event handlers, request-response pattern
- l2ps_types: full protocol types (client/server messages, encryption, storage, errors)
- L2PSInstantMessagingPayload: transaction payload type for L2PS-backed messaging
- Updated Transaction union type and exports

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 18, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Adds a new L2PS WebSocket instant-messaging client and protocol types, re-exports, and related blockchain payload/transaction types; implements registration, send/history/discover/requestPublicKey flows, event handlers, reconnection, request/response waiters, and outgoing frame queuing.

Changes

Cohort / File(s) Summary
L2PS Messaging Peer Implementation
src/instant_messaging/L2PSMessagingPeer.ts
New WebSocket client class L2PSMessagingPeer with L2PSMessagingConfig, lifecycle (connect/disconnect), registration signing, send/history/discover/requestPublicKey APIs, pending-response waiters with timeouts, outgoing queueing, reconnection/backoff, peer state, and event handler registration/removal.
L2PS Protocol Type Definitions
src/instant_messaging/l2ps_types.ts
New comprehensive protocol typings: MessageEnvelope, client/server frame unions (ClientMessageType/ServerMessageType), concrete request/response/notification interfaces, SerializedEncryptedMessage, StoredMessage, MessageStatus, and ErrorCode.
Instant Messaging Exports
src/instant_messaging/index.ts
Re-exports L2PSMessagingPeer and related handler/config types; re-exports all from l2ps_types; adds usage comments about legacy signaling.
Blockchain Transaction Types
src/types/blockchain/Transaction.ts, src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts, src/types/blockchain/TransactionSubtypes/index.ts
Extended transaction unions to include l2psInstantMessaging content/data; added L2PSInstantMessagingTransactionContent and L2PSInstantMessagingTransaction; updated SpecificTransaction union.
L2PS Payload Type
src/types/instantMessaging/index.ts
Added L2PSInstantMessagingPayload interface describing L2PS transaction payload (messageId, messageHash, encrypted block, timestamp).
Repo/config/docs edits
.mycelium/.gitignore, .serena/project.yml, AGENTS.md
Minor additions: .linear/ to gitignore, new ls_specific_settings and ignored_memory_patterns keys in project config, and documentation edits adding "Mental Frameworks for Mycelium Usage".

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client Code
    participant Peer as L2PSMessagingPeer
    participant WS as WebSocket Server
    participant Queue as Outgoing Queue

    Client->>Peer: connect()
    activate Peer
    Peer->>WS: open WebSocket(serverUrl)
    WS-->>Peer: onopen
    Peer->>Peer: sign register proof
    Peer->>WS: RegisterMessage
    WS-->>Peer: RegisteredResponse (peers)
    Peer-->>Client: connect() resolves
    deactivate Peer

    Client->>Peer: send(to, encrypted, messageHash)
    activate Peer
    Peer->>Queue: enqueue if socket not ready
    Peer->>WS: SendMessage
    Peer->>Peer: await [message_sent | message_queued]
    WS-->>Peer: MessageSentResponse / MessageQueuedResponse
    Peer-->>Client: send() resolves
    deactivate Peer

    WS->>Peer: IncomingMessage
    activate Peer
    Peer->>Peer: dispatch to message handlers
    deactivate Peer
Loading
sequenceDiagram
    participant Client as Client Code
    participant Peer as L2PSMessagingPeer
    participant WS as WebSocket Server

    Client->>Peer: discover()
    activate Peer
    Peer->>WS: DiscoverMessage
    Peer->>Peer: await DiscoverResponse
    WS-->>Peer: DiscoverResponse (peer list)
    Peer-->>Client: discover() resolves
    deactivate Peer

    WS->>Peer: PeerJoinedNotification
    activate Peer
    Peer->>Peer: update peers set
    Peer-->>Client: invoke onPeerJoined handlers
    deactivate Peer
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Poem

🐰 I signed a note and hopped online,
Queued my frames, waited for the sign,
Peers come, peers go, I stitch the thread,
Reconnect, resend — no message left unread,
Hooray, the rabbits chat in time.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat: add L2PS messaging client and protocol types' clearly and specifically summarizes the main changes: introduction of L2PSMessagingPeer client and l2ps_types protocol definitions.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/l2ps-messaging-types

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link
Copy Markdown
Contributor

Review Summary by Qodo

Add L2PS messaging client and protocol types

✨ Enhancement

Grey Divider

Walkthroughs

Description
• Add L2PSMessagingPeer WebSocket client with auto-reconnect and event handlers
• Define comprehensive L2PS messaging protocol types for client-server communication
• Add L2PSInstantMessagingPayload transaction type for L2PS-backed messaging
• Update Transaction union types to support L2PS instant messaging transactions
Diagram
flowchart LR
  A["L2PSMessagingPeer<br/>WebSocket Client"] -->|"sends/receives"| B["l2ps_types<br/>Protocol Types"]
  B -->|"defines"| C["Message Envelope<br/>Encryption Types"]
  A -->|"handles"| D["Connection<br/>Registration<br/>Messaging"]
  E["L2PSInstantMessagingPayload"] -->|"extends"| F["Transaction Types"]
  A -.->|"uses"| E
Loading

Grey Divider

File Changes

1. src/instant_messaging/L2PSMessagingPeer.ts ✨ Enhancement +514/-0

L2PS WebSocket messaging client implementation

• Implements WebSocket client for L2PS messaging server with auto-reconnect logic
• Provides registration with ed25519 proof and L2PS network isolation
• Supports E2E encrypted message sending/receiving with request-response pattern
• Implements conversation history retrieval, peer discovery, and offline message queuing
• Includes event handlers for messages, errors, peer joins/leaves, and connection state changes

src/instant_messaging/L2PSMessagingPeer.ts


2. src/instant_messaging/l2ps_types.ts ✨ Enhancement +246/-0

L2PS messaging protocol type definitions

• Defines MessageEnvelope structure with ed25519 signatures and encryption support
• Specifies ClientMessageType and ServerMessageType enums for protocol communication
• Implements protocol frame types for register, send, history, discover, and public key requests
• Defines response types including RegisteredResponse, IncomingMessage, and HistoryResponse
• Includes encryption types (SerializedEncryptedMessage) and storage types (StoredMessage,
 MessageStatus)
• Defines error codes for protocol error handling

src/instant_messaging/l2ps_types.ts


3. src/instant_messaging/index.ts ✨ Enhancement +13/-0

Export L2PS messaging client and types

• Exports L2PSMessagingPeer class and related handler types
• Re-exports all types from l2ps_types module
• Adds clarifying comment about legacy signaling server vs L2PS messaging

src/instant_messaging/index.ts


View more (4)
4. src/types/blockchain/Transaction.ts ✨ Enhancement +2/-1

Add L2PS instant messaging to transaction union

• Imports L2PSInstantMessagingPayload from instantMessaging types
• Adds L2PSInstantMessagingPayload as alternative transaction content type
• Maintains backward compatibility with existing InstantMessagingPayload

src/types/blockchain/Transaction.ts


5. src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts ✨ Enhancement +12/-2

Add L2PS instant messaging transaction types

• Imports L2PSInstantMessagingPayload type
• Adds L2PSInstantMessagingTransactionContent type for L2PS-backed messaging
• Adds L2PSInstantMessagingTransaction interface extending base Transaction
• Maintains existing InstantMessagingTransaction for legacy messaging

src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts


6. src/types/blockchain/TransactionSubtypes/index.ts ✨ Enhancement +2/-1

Export L2PS instant messaging transaction type

• Imports L2PSInstantMessagingTransaction from InstantMessagingTransaction module
• Adds L2PSInstantMessagingTransaction to SpecificTransaction union type
• Maintains existing InstantMessagingTransaction in union

src/types/blockchain/TransactionSubtypes/index.ts


7. src/types/instantMessaging/index.ts ✨ Enhancement +19/-0

Add L2PS instant messaging payload type

• Adds L2PSInstantMessagingPayload interface with messageId, messageHash, encrypted data, and
 timestamp
• Defines encrypted message structure with ciphertext, nonce, and optional ephemeralKey
• Maintains existing InstantMessagingPayload for backward compatibility

src/types/instantMessaging/index.ts


Grey Divider

Qodo Logo

@qodo-code-review
Copy link
Copy Markdown
Contributor

qodo-code-review bot commented Mar 18, 2026

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0) 📐 Spec deviations (0)

Grey Divider


Action required

1. Connect interval leaks🐞 Bug ⛯ Reliability
Description
L2PSMessagingPeer.connect() starts a polling setInterval but never clears it when the 10s timeout
rejects, leaving a live interval and allowing background connect/register after connect() already
failed. This can cause resource leaks and unexpected registered/connected state after a failed
connect attempt.
Code

src/instant_messaging/L2PSMessagingPeer.ts[R119-143]

+    async connect(): Promise<RegisteredResponse["payload"]> {
+        return new Promise((resolve, reject) => {
+            const timeout = setTimeout(() => {
+                reject(new Error("Connection timeout (10s)"))
+            }, 10000)
+
+            this.shouldReconnect = true
+            this.connectWebSocket()
+
+            // Wait for WS open, then register
+            const checkOpen = setInterval(() => {
+                if (this._isConnected) {
+                    clearInterval(checkOpen)
+                    this.register()
+                        .then(response => {
+                            clearTimeout(timeout)
+                            resolve(response)
+                        })
+                        .catch(err => {
+                            clearTimeout(timeout)
+                            reject(err)
+                        })
+                }
+            }, 50)
+        })
Evidence
The interval is only cleared on successful connection; the timeout path rejects without clearing the
interval or stopping reconnection, so the poll continues indefinitely and may trigger register
later.

src/instant_messaging/L2PSMessagingPeer.ts[119-143]
src/instant_messaging/L2PSMessagingPeer.ts[342-350]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`L2PSMessagingPeer.connect()` creates a polling `setInterval` (`checkOpen`) and a 10s `setTimeout` that rejects the promise. If the timeout fires first, the promise rejects but the interval continues running; additionally, the websocket can keep reconnecting and eventually call `register()` in the background.
## Issue Context
This causes a resource leak (interval runs forever) and confusing behavior (caller sees a failure but the peer may later become registered/connected).
## Fix Focus Areas
- src/instant_messaging/L2PSMessagingPeer.ts[119-143]
## Implementation notes
- Store `checkOpen` in a variable accessible to the timeout handler.
- In the timeout handler: `clearInterval(checkOpen)` and consider disabling reconnection/closing the socket (e.g., set `shouldReconnect = false` and `ws?.close()`) so the client doesn’t later register unexpectedly.
- Also ensure `checkOpen` is cleared on *any* resolve/reject path (including register failure).

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Waiter key collisions🐞 Bug ✓ Correctness
Description
waitForResponse() overwrites existing pendingResponses entries for the same key without cancelling
the old timer; when the old timer fires it deletes the newer entry and rejects the wrong promise.
This breaks concurrent operations that reuse keys (e.g., multiple discover() calls).
Code

src/instant_messaging/L2PSMessagingPeer.ts[R240-253]

+    async discover(): Promise<string[]> {
+        this.ensureRegistered()
+
+        this.sendFrame({
+            type: "discover",
+            payload: {},
+            timestamp: Date.now(),
+        })
+
+        const response = await this.waitForResponse<DiscoverResponse["payload"]>(
+            "discover",
+            ["discover_response"],
+            10000,
+        )
Evidence
discover() uses a constant key "discover" and waitForResponse() does a blind Map.set(key, entry); a
second discover() replaces the first entry. The first timer later executes
pendingResponses.delete(key), which deletes the *second* entry, leaving it without a waiter and
causing hard-to-debug timeouts.

src/instant_messaging/L2PSMessagingPeer.ts[240-253]
src/instant_messaging/L2PSMessagingPeer.ts[454-470]
src/instant_messaging/L2PSMessagingPeer.ts[208-234]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`waitForResponse(key, ...)` uses `pendingResponses.set(key, entry)` without checking for an existing entry with the same key. If two calls reuse the same key concurrently (notably `discover()` uses constant key `&quot;discover&quot;`), the second overwrites the first. The first timer callback will later delete the key (removing the second entry) and reject the wrong promise.
## Issue Context
This is a correctness bug that can cause hanging promises and spurious timeouts in concurrent usage.
## Fix Focus Areas
- src/instant_messaging/L2PSMessagingPeer.ts[240-253]
- src/instant_messaging/L2PSMessagingPeer.ts[208-234]
- src/instant_messaging/L2PSMessagingPeer.ts[454-470]
## Implementation notes (choose one)
1) **Disallow collisions**: if `pendingResponses.has(key)`, immediately reject the new call with a clear error (e.g., `Discover already in progress`).
2) **Make keys unique**: incorporate a monotonically increasing counter / timestamp into the key for `discover()` and `history()` and ensure matching uses a filter function that can distinguish responses (only possible if the response payload contains an echoable identifier).
At minimum, implement option (1) to avoid corrupting the map/timers.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. IM transaction not discriminable🐞 Bug ⚙ Maintainability
Description
The PR adds a second instantMessaging transaction/payload variant but keeps the same transaction
content.type and the same content.data[0] discriminator ("instantMessaging") for both legacy and
L2PS-backed formats. This makes routing and type-guarding ambiguous and can lead to incorrect
handling at compile time and runtime.
Code

src/types/blockchain/Transaction.ts[R38-42]

 | ["l2psEncryptedTx", L2PSEncryptedPayload]
 | ["identity", IdentityPayload]
 | ["instantMessaging", InstantMessagingPayload]
+    | ["instantMessaging", L2PSInstantMessagingPayload]
 | ["nativeBridge", NativeBridgeTxPayload]
Evidence
TransactionContentData now contains two tuple variants with identical first element
"instantMessaging". Existing utilities type-guard by tx.content.data[0], which can no longer
distinguish legacy vs L2PS payloads, and both subtype interfaces also keep `type:
'instantMessaging' and data: ['instantMessaging', ...]`.

src/types/blockchain/Transaction.ts[33-44]
src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts[4-21]
src/types/blockchain/TransactionSubtypes/utils.ts[42-47]
src/types/instantMessaging/index.ts[1-27]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Legacy instant messaging and L2PS-backed instant messaging are represented with the same transaction discriminator values:
- `content.type === &#x27;instantMessaging&#x27;`
- `content.data[0] === &#x27;instantMessaging&#x27;`
This makes them indistinguishable for transaction routing and for `isTransactionDataType()` which checks only `data[0]`.
## Issue Context
Other transaction variants use unique discriminators (e.g., `l2psEncryptedTx`). Instant messaging now has two incompatible payload shapes under the same discriminator.
## Fix Focus Areas
- src/types/blockchain/Transaction.ts[33-44]
- src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts[4-21]
- src/types/blockchain/TransactionSubtypes/utils.ts[42-47]
- src/types/instantMessaging/index.ts[1-27]
## Implementation notes
- Introduce a distinct tuple key and/or `content.type` for the L2PS-backed variant (e.g., `[&#x27;l2psInstantMessaging&#x27;, L2PSInstantMessagingPayload]` and `type: &#x27;l2psInstantMessaging&#x27;`).
- Update subtype interfaces and any type guards accordingly.
- If wire/backward-compat constraints require `content.type` to stay `instantMessaging`, at least make `content.data[0]` unique so `isTransactionDataType()` can discriminate safely.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

4. L2PS IM payload not exported 🐞 Bug ⚙ Maintainability
Description
L2PSInstantMessagingPayload is added but not re-exported from the main src/types/index.ts barrel,
so consumers importing from @/types can’t access the new payload type even though it’s now
referenced by TransactionContentData. This makes it harder/impossible for SDK users to construct
strongly-typed L2PS instant messaging transactions via the primary types entrypoint.
Code

src/types/instantMessaging/index.ts[R10-27]

+/** L2PS-backed instant messaging transaction payload */
+export interface L2PSInstantMessagingPayload {
+    type: "instantMessaging"
+    data: {
+        /** UUID v4 message identifier */
+        messageId: string
+        /** SHA256 hash for dedup */
+        messageHash: string
+        /** E2E encrypted message (serialized) */
+        encrypted: {
+            ciphertext: string
+            nonce: string
+            ephemeralKey?: string
+        }
+        /** Unix timestamp (ms) */
+        timestamp: number
+    }
+}
Evidence
The new payload interface exists in src/types/instantMessaging/index.ts, but the main types barrel
only exports InstantMessagingPayload, not the new L2PSInstantMessagingPayload.

src/types/instantMessaging/index.ts[10-27]
src/types/index.ts[61-64]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The SDK’s main types barrel (`src/types/index.ts`) does not re-export the newly introduced `L2PSInstantMessagingPayload`, so users importing from `@/types` cannot reference it.
## Issue Context
`TransactionContentData` now includes `L2PSInstantMessagingPayload`, so downstream code may reasonably expect this type to be available from the primary types entrypoint.
## Fix Focus Areas
- src/types/index.ts[61-64]
## Implementation notes
- Update the exports to include `L2PSInstantMessagingPayload` (e.g., `export { InstantMessagingPayload, L2PSInstantMessagingPayload } from &#x27;./instantMessaging&#x27;`), or switch to `export * from &#x27;./instantMessaging&#x27;` if that matches project conventions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (2)
src/instant_messaging/index.ts (1)

4-12: Use @/ aliases for the new barrel re-exports.

These new exports are still wired through relative paths. Switching them to the repository alias keeps this public entrypoint aligned with the SDK import convention.

♻️ Proposed change
-export { L2PSMessagingPeer } from "./L2PSMessagingPeer"
+export { L2PSMessagingPeer } from "@/instant_messaging/L2PSMessagingPeer"
 export type {
     L2PSMessagingConfig,
     L2PSMessageHandler,
     L2PSErrorHandler,
     L2PSPeerHandler,
     L2PSConnectionStateHandler,
-} from "./L2PSMessagingPeer"
-export * from "./l2ps_types"
+} from "@/instant_messaging/L2PSMessagingPeer"
+export * from "@/instant_messaging/l2ps_types"

As per coding guidelines, "Use @/ path aliases instead of relative imports".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/index.ts` around lines 4 - 12, The current barrel in
index.ts exports L2PSMessagingPeer and related types via relative paths; update
these to use the repository path alias (e.g., replace "./L2PSMessagingPeer" and
"./l2ps_types" with the corresponding "@/..." alias) so the public entrypoint
follows the SDK import convention; ensure the exported symbols
L2PSMessagingPeer, L2PSMessagingConfig, L2PSMessageHandler, L2PSErrorHandler,
L2PSPeerHandler, L2PSConnectionStateHandler and the re-export of l2ps_types all
reference the alias form.
src/instant_messaging/L2PSMessagingPeer.ts (1)

57-80: Model IncomingFrame as a discriminated union and type the pending metadata storage instead of using as any casts.

IncomingFrame.payload, pendingResponses value types, and the _meta attachment patterns all use any, bypassing type checking where frames are parsed and matched. Replace IncomingFrame with a union of the imported server response types (e.g., RegisteredResponse | IncomingMessage | MessageSentResponse | ...), and refactor the pending entry storage to include a properly typed meta property instead of mutating via (entry as any)._meta.

Per coding guidelines: "Ensure full TypeScript type coverage."

Also applies to: 416–469

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 57 - 80, Replace the
ad-hoc IncomingFrame and the any-typed pendingResponses values with proper typed
constructs: change IncomingFrame to a discriminated union of the actual server
response interfaces (e.g., RegisteredResponse | IncomingMessage |
MessageSentResponse | ... using the existing ServerMessageType discriminant),
and update the pendingResponses Map value type to an interface that includes
resolve, reject, timer and a typed meta property (e.g., meta?: PendingMetaType)
so you stop using (entry as any)._meta; then update parsing/dispatch logic in
L2PSMessagingPeer where frames are parsed and where pendingResponses entries are
created/mutated to use entry.meta (with the correct type) and pattern-match on
the discriminant to narrow payload types instead of treating payload as any.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/instant_messaging/l2ps_types.ts`:
- Around line 65-70: Update the wire frame types to carry a request correlation
id and make the request/response/error frames include it: add a required
requestId: string property to the payload shape used for request-response flows
(or add requestId: string directly on ProtocolFrame where you define
request/response/error variants) so messages round-trip with a stable id; then
update L2PSMessagingPeer.waitForResponse() to match incoming frames by that
requestId (see L2PSMessagingPeer.waitForResponse and the request/response/error
shapes currently guessed as "discover" or "history:${peerKey}" and the
ProtocolFrame<T> interface in l2ps_types.ts) so concurrent/late responses and
error frames can be correlated reliably.

In `@src/instant_messaging/L2PSMessagingPeer.ts`:
- Around line 119-143: The connect() promise can time out but leave the
checkOpen interval running, allowing register() to run later; fix by having the
timeout handler also clearInterval(checkOpen) and stop reconnection attempts
(e.g. set this.shouldReconnect = false and/or close the socket) so no further
register() is triggered after rejection; update the timeout callback in
connect() to clear the checkOpen interval and disable reconnection before
rejecting, and keep the existing register() path unchanged.
- Around line 335-349: onclose currently unsets _isRegistered but onopen simply
sets _isConnected and calls flushQueue, which leaves the peer permanently
unregistered and causes ensureRegistered() to fail; change the onopen handler
(and/or the reconnect flow in attemptReconnect/flushQueue) to attempt
re-registration (call the same registration logic used initially) before calling
notifyConnectionState("connected") and flushing queued frames, and if
re-registration fails close the socket and trigger attemptReconnect() (or retry
with backoff) instead of marking connected; ensure you reference and update
_isRegistered only after successful registration and use
notifyConnectionState("disconnected") when closing for retries.

In `@src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts`:
- Around line 13-20: The two transaction variants share the same discriminators
so runtime narrowing fails; change the L2PS-specific discriminators to unique
values (e.g. update L2PSInstantMessagingTransactionContent.type and its data[0]
from 'instantMessaging' to a unique token like 'l2psInstantMessaging') and
update any corresponding entries in the TransactionContentData union (in
Transaction.ts) so the union contains the new L2PS discriminator; ensure helpers
in src/types/blockchain/TransactionSubtypes/utils.ts that compare content.type
and data[0] will now correctly distinguish the L2PS variant from the non-L2PS
InstantMessaging variant.

---

Nitpick comments:
In `@src/instant_messaging/index.ts`:
- Around line 4-12: The current barrel in index.ts exports L2PSMessagingPeer and
related types via relative paths; update these to use the repository path alias
(e.g., replace "./L2PSMessagingPeer" and "./l2ps_types" with the corresponding
"@/..." alias) so the public entrypoint follows the SDK import convention;
ensure the exported symbols L2PSMessagingPeer, L2PSMessagingConfig,
L2PSMessageHandler, L2PSErrorHandler, L2PSPeerHandler,
L2PSConnectionStateHandler and the re-export of l2ps_types all reference the
alias form.

In `@src/instant_messaging/L2PSMessagingPeer.ts`:
- Around line 57-80: Replace the ad-hoc IncomingFrame and the any-typed
pendingResponses values with proper typed constructs: change IncomingFrame to a
discriminated union of the actual server response interfaces (e.g.,
RegisteredResponse | IncomingMessage | MessageSentResponse | ... using the
existing ServerMessageType discriminant), and update the pendingResponses Map
value type to an interface that includes resolve, reject, timer and a typed meta
property (e.g., meta?: PendingMetaType) so you stop using (entry as any)._meta;
then update parsing/dispatch logic in L2PSMessagingPeer where frames are parsed
and where pendingResponses entries are created/mutated to use entry.meta (with
the correct type) and pattern-match on the discriminant to narrow payload types
instead of treating payload as any.
🪄 Autofix (Beta)

✅ Autofix completed


ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5788318b-c75a-49d1-86f5-306a375d5da6

📥 Commits

Reviewing files that changed from the base of the PR and between 4b8ba5f and 72f3e27.

📒 Files selected for processing (7)
  • src/instant_messaging/L2PSMessagingPeer.ts
  • src/instant_messaging/index.ts
  • src/instant_messaging/l2ps_types.ts
  • src/types/blockchain/Transaction.ts
  • src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts
  • src/types/blockchain/TransactionSubtypes/index.ts
  • src/types/instantMessaging/index.ts

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 26, 2026

Note

Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it.

Fixes Applied Successfully

Fixed 4 file(s) based on 4 unresolved review comments.

Files modified:

  • src/instant_messaging/L2PSMessagingPeer.ts
  • src/instant_messaging/l2ps_types.ts
  • src/types/blockchain/Transaction.ts
  • src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts

Commit: a9ac32124145c8ef8dafb66b0997307ff48d7e83

The changes have been pushed to the feat/l2ps-messaging-types branch.

Time taken: 10m 8s

Fixed 4 file(s) based on 4 unresolved review comments.

Co-authored-by: CodeRabbit <[email protected]>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (6)
src/instant_messaging/L2PSMessagingPeer.ts (5)

165-170: Remove unused loop variable.

The key variable is extracted but never used. Either prefix it with underscore to indicate intentional discard, or iterate over values only.

🧹 Proposed fix
         // Reject all pending responses
-        for (const [key, pending] of this.pendingResponses) {
+        for (const [_key, pending] of this.pendingResponses) {
             clearTimeout(pending.timer)
             pending.reject(new Error("Disconnected"))
         }

Or simpler:

         // Reject all pending responses
-        for (const [key, pending] of this.pendingResponses) {
+        for (const pending of this.pendingResponses.values()) {
             clearTimeout(pending.timer)
             pending.reject(new Error("Disconnected"))
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 165 - 170, The loop
over this.pendingResponses extracts an unused key; update the iteration to avoid
the unused variable by iterating values only (e.g., use
this.pendingResponses.values() or destructure to skip the key with an
underscore) so you still call clearTimeout(pending.timer) and pending.reject(new
Error("Disconnected")) and then call this.pendingResponses.clear(); keep
references to pending.timer and pending.reject intact.

13-29: Remove unused imports and consider using path alias.

Static analysis indicates StoredMessage, PeerJoinedNotification, and PeerLeftNotification are imported but never used. Additionally, per coding guidelines, prefer @/ path aliases over relative imports.

🧹 Proposed fix
 import type {
     SerializedEncryptedMessage,
     ClientMessageType,
     ServerMessageType,
     RegisteredResponse,
     IncomingMessage,
     MessageSentResponse,
     MessageQueuedResponse,
     HistoryResponse,
     DiscoverResponse,
     PublicKeyResponse,
-    PeerJoinedNotification,
-    PeerLeftNotification,
     ErrorResponse,
     ErrorCode,
-    StoredMessage,
-} from "./l2ps_types"
+} from "@/instant_messaging/l2ps_types"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 13 - 29, Remove the
unused imports StoredMessage, PeerJoinedNotification, and PeerLeftNotification
from the import list in L2PSMessagingPeer (they are not referenced anywhere in
this file) and update the module import path from the relative "./l2ps_types" to
the project path alias "@/instant_messaging/l2ps_types" (or the appropriate
"@/..." alias used in the repo) so the import uses the canonical alias per
guidelines; ensure only the actually used types (e.g.,
SerializedEncryptedMessage, ClientMessageType, ServerMessageType,
RegisteredResponse, IncomingMessage, MessageSentResponse, MessageQueuedResponse,
HistoryResponse, DiscoverResponse, PublicKeyResponse, ErrorResponse, ErrorCode)
remain in the import statement.

78-82: Consider integrating PendingMeta into the pending response type.

Using (entry as any)._meta to attach metadata is fragile. Integrating the metadata into the Map's value type would improve type safety.

🧹 Proposed fix
+interface PendingResponseEntry {
+    resolve: (value: any) => void
+    reject: (error: Error) => void
+    timer: NodeJS.Timeout
+    expectedTypes: ServerMessageType[]
+}

     // Pending request-response waiters
-    private pendingResponses: Map<
-        string,
-        { resolve: (value: any) => void; reject: (error: Error) => void; timer: NodeJS.Timeout }
-    > = new Map()
+    private readonly pendingResponses: Map<string, PendingResponseEntry> = new Map()

Then update waitForResponse:

-            const entry = { resolve, reject, timer }
-            // Attach metadata for matching
-            ;(entry as any)._meta = { types: expectedTypes } as PendingMeta
-            this.pendingResponses.set(requestId, entry)
+            this.pendingResponses.set(requestId, { resolve, reject, timer, expectedTypes })

And handleFrame:

-            const meta = (pending as any)._meta as PendingMeta | undefined
-            if (meta?.types.includes(frame.type)) {
+            if (pending.expectedTypes.includes(frame.type)) {

Also applies to: 516-519

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 78 - 82, The
pendingResponses Map currently stores value objects without typed metadata and
code uses (entry as any)._meta; introduce a PendingMeta type and change the Map
value type to include meta, e.g. { resolve, reject, timer, meta: PendingMeta },
then update any places that create or read entries (notably waitForResponse and
handleFrame) to set and read entry.meta instead of using (entry as any)._meta so
the metadata is strongly typed and you can drop the casts; also update other
similar maps/usages around lines referenced (516-519) to the same typed shape.

459-479: Simplify frame handling with optional chaining and remove unnecessary assertions.

The non-null assertions after .has() checks are redundant. Use optional chaining for cleaner code.

🧹 Proposed fix
     private handleFrame(frame: IncomingFrame): void {
         // First, check if any pending response matches by requestId
-        if (frame.requestId && this.pendingResponses.has(frame.requestId)) {
-            const pending = this.pendingResponses.get(frame.requestId)!
+        if (frame.requestId) {
+            const pending = this.pendingResponses.get(frame.requestId)
+            if (!pending) {
+                // No waiter, fall through to event dispatch
+            } else {
             const meta = (pending as any)._meta as PendingMeta | undefined
-            if (meta && meta.types.includes(frame.type)) {
+            if (meta?.types.includes(frame.type)) {
                 clearTimeout(pending.timer)
                 this.pendingResponses.delete(frame.requestId)
                 pending.resolve(frame.payload)
                 return
             }
+            }
         }

         // Handle error frames with requestId
-        if (frame.type === "error" && frame.requestId && this.pendingResponses.has(frame.requestId)) {
-            const pending = this.pendingResponses.get(frame.requestId)!
+        if (frame.type === "error" && frame.requestId) {
+            const pending = this.pendingResponses.get(frame.requestId)
+            if (pending) {
             clearTimeout(pending.timer)
             this.pendingResponses.delete(frame.requestId)
             pending.reject(new Error(frame.payload?.message || "Server error"))
             return
+            }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 459 - 479, In
handleFrame, remove redundant non-null assertions after Map.has() by using
optional chaining and early-returns: when checking
this.pendingResponses.has(frame.requestId) replace the get(...)! usage with
const pending = this.pendingResponses.get(frame.requestId); guard with if
(!pending) return; use pending?._meta and pending?.timer etc.; similarly in the
error frame block get the pending via get and guard instead of using !, and use
optional chaining for frame.payload?.message when constructing the Error; ensure
you still clearTimeout, delete from this.pendingResponses, and call
pending.resolve/pending.reject as before.

68-96: Mark non-reassigned members as readonly.

Static analysis correctly identifies that several class members are never reassigned. Marking them readonly prevents accidental mutation and clarifies intent.

🧹 Proposed fix
 export class L2PSMessagingPeer {
     private ws: WebSocket | null = null
-    private config: L2PSMessagingConfig
+    private readonly config: L2PSMessagingConfig

     // Event handlers
-    private messageHandlers: Set<L2PSMessageHandler> = new Set()
-    private errorHandlers: Set<L2PSErrorHandler> = new Set()
-    private peerJoinedHandlers: Set<L2PSPeerHandler> = new Set()
-    private peerLeftHandlers: Set<L2PSPeerHandler> = new Set()
-    private connectionStateHandlers: Set<L2PSConnectionStateHandler> = new Set()
+    private readonly messageHandlers: Set<L2PSMessageHandler> = new Set()
+    private readonly errorHandlers: Set<L2PSErrorHandler> = new Set()
+    private readonly peerJoinedHandlers: Set<L2PSPeerHandler> = new Set()
+    private readonly peerLeftHandlers: Set<L2PSPeerHandler> = new Set()
+    private readonly connectionStateHandlers: Set<L2PSConnectionStateHandler> = new Set()

     // Pending request-response waiters
-    private pendingResponses: Map<
+    private readonly pendingResponses: Map<
         string,
         { resolve: (value: any) => void; reject: (error: Error) => void; timer: NodeJS.Timeout }
     > = new Map()

     // State
     private _isConnected = false
     private _isRegistered = false
     private onlinePeers: Set<string> = new Set()
-    private messageQueue: OutgoingFrame[] = []
+    private readonly messageQueue: OutgoingFrame[] = []

     // Reconnection
     private reconnectAttempts = 0
-    private maxReconnectAttempts = 10
-    private baseReconnectDelay = 1000
+    private readonly maxReconnectAttempts = 10
+    private readonly baseReconnectDelay = 1000
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 68 - 96, Several
instance fields in L2PSMessagingPeer are never reassigned; mark them readonly to
prevent accidental reassignment. Update the declarations in class
L2PSMessagingPeer for config, messageHandlers, errorHandlers,
peerJoinedHandlers, peerLeftHandlers, connectionStateHandlers, pendingResponses,
onlinePeers, messageQueue, baseReconnectDelay, and maxReconnectAttempts to
include the readonly modifier (leave fields that are mutated such as ws,
_isConnected, _isRegistered, reconnectAttempts, reconnectTimeout,
shouldReconnect, isReconnecting unchanged). This keeps existing mutation of
collection contents intact while preventing reassignment of the references.
src/instant_messaging/l2ps_types.ts (1)

229-237: Consider adding a "read" status for delivery receipts.

The MessageStatus type covers the message lifecycle well, but lacks a status for when the recipient has read the message. If read receipts are planned, consider adding this now to avoid a breaking change later.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/l2ps_types.ts` around lines 229 - 237, The
MessageStatus union type is missing a "read" state for delivery receipts; update
the MessageStatus type definition (the exported type named MessageStatus in
l2ps_types.ts) to include a "read" variant (e.g., | "read") and ensure any
switch/case, serializers, database enums, and tests that consume MessageStatus
are updated to handle the new "read" value so this addition won't break type
narrowing elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/instant_messaging/L2PSMessagingPeer.ts`:
- Around line 559-561: The generateRequestId function uses the deprecated
String.prototype.substr; replace that call in generateRequestId with a
non-deprecated alternative (e.g., Math.random().toString(36).substring(2, 11) or
.slice(2, 11)) to preserve the same 9-character extraction, so update the return
expression in generateRequestId to use substring (or slice) instead of substr.
- Around line 392-396: The WebSocket error handler currently passes useless
details via String(event); update the this.ws.onerror callback (the handler that
invokes this.errorHandlers) to provide meaningful information by extracting
event.type when available or event.message if event is an Error, and fall back
to a safe JSON serialization (e.g., JSON.stringify with try/catch) or an empty
string; ensure the ErrorCode stays "INTERNAL_ERROR" and that the message remains
"WebSocket error" while replacing details: String(event) with the
extracted/serialized value.
- Around line 363-374: The catch block that handles re-registration failures
currently clears state, closes the socket and triggers reconnection but swallows
the error; update it to notify consumers by invoking an error-notification path
(e.g., call this.notifyError(err) or emit an 'error' event) before/after calling
notifyConnectionState("disconnected") so handlers can react and debug, and if
notifyError/emit isn't implemented add a small helper (e.g., notifyError) that
forwards the error to registered listeners; keep the existing state changes and
the existing calls to this.ws.close() and this.attemptReconnect().

---

Nitpick comments:
In `@src/instant_messaging/l2ps_types.ts`:
- Around line 229-237: The MessageStatus union type is missing a "read" state
for delivery receipts; update the MessageStatus type definition (the exported
type named MessageStatus in l2ps_types.ts) to include a "read" variant (e.g., |
"read") and ensure any switch/case, serializers, database enums, and tests that
consume MessageStatus are updated to handle the new "read" value so this
addition won't break type narrowing elsewhere.

In `@src/instant_messaging/L2PSMessagingPeer.ts`:
- Around line 165-170: The loop over this.pendingResponses extracts an unused
key; update the iteration to avoid the unused variable by iterating values only
(e.g., use this.pendingResponses.values() or destructure to skip the key with an
underscore) so you still call clearTimeout(pending.timer) and pending.reject(new
Error("Disconnected")) and then call this.pendingResponses.clear(); keep
references to pending.timer and pending.reject intact.
- Around line 13-29: Remove the unused imports StoredMessage,
PeerJoinedNotification, and PeerLeftNotification from the import list in
L2PSMessagingPeer (they are not referenced anywhere in this file) and update the
module import path from the relative "./l2ps_types" to the project path alias
"@/instant_messaging/l2ps_types" (or the appropriate "@/..." alias used in the
repo) so the import uses the canonical alias per guidelines; ensure only the
actually used types (e.g., SerializedEncryptedMessage, ClientMessageType,
ServerMessageType, RegisteredResponse, IncomingMessage, MessageSentResponse,
MessageQueuedResponse, HistoryResponse, DiscoverResponse, PublicKeyResponse,
ErrorResponse, ErrorCode) remain in the import statement.
- Around line 78-82: The pendingResponses Map currently stores value objects
without typed metadata and code uses (entry as any)._meta; introduce a
PendingMeta type and change the Map value type to include meta, e.g. { resolve,
reject, timer, meta: PendingMeta }, then update any places that create or read
entries (notably waitForResponse and handleFrame) to set and read entry.meta
instead of using (entry as any)._meta so the metadata is strongly typed and you
can drop the casts; also update other similar maps/usages around lines
referenced (516-519) to the same typed shape.
- Around line 459-479: In handleFrame, remove redundant non-null assertions
after Map.has() by using optional chaining and early-returns: when checking
this.pendingResponses.has(frame.requestId) replace the get(...)! usage with
const pending = this.pendingResponses.get(frame.requestId); guard with if
(!pending) return; use pending?._meta and pending?.timer etc.; similarly in the
error frame block get the pending via get and guard instead of using !, and use
optional chaining for frame.payload?.message when constructing the Error; ensure
you still clearTimeout, delete from this.pendingResponses, and call
pending.resolve/pending.reject as before.
- Around line 68-96: Several instance fields in L2PSMessagingPeer are never
reassigned; mark them readonly to prevent accidental reassignment. Update the
declarations in class L2PSMessagingPeer for config, messageHandlers,
errorHandlers, peerJoinedHandlers, peerLeftHandlers, connectionStateHandlers,
pendingResponses, onlinePeers, messageQueue, baseReconnectDelay, and
maxReconnectAttempts to include the readonly modifier (leave fields that are
mutated such as ws, _isConnected, _isRegistered, reconnectAttempts,
reconnectTimeout, shouldReconnect, isReconnecting unchanged). This keeps
existing mutation of collection contents intact while preventing reassignment of
the references.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 68f5edc0-dabf-49ee-a68d-1c45f5eeb70a

📥 Commits

Reviewing files that changed from the base of the PR and between 72f3e27 and a9ac321.

📒 Files selected for processing (4)
  • src/instant_messaging/L2PSMessagingPeer.ts
  • src/instant_messaging/l2ps_types.ts
  • src/types/blockchain/Transaction.ts
  • src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/types/blockchain/Transaction.ts
  • src/types/blockchain/TransactionSubtypes/InstantMessagingTransaction.ts

tcsenpai added 3 commits March 26, 2026 16:50
- Replace IncomingFrame.payload: any with ServerPayload discriminated union
- Eliminate (entry as any)._meta casts by typing pendingResponses map properly
- Add type-narrowing casts in handleFrame switch branches
- Notify error handlers when re-registration fails after reconnect
- Fix WebSocket error stringification (String(event) → proper extraction)
- Replace deprecated substr() with substring()
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (6)
AGENTS.md (1)

244-246: Add a language specifier to the fenced code block.

The code block lacks a language identifier. Since this is a pseudo-code formula, consider using text or plaintext to satisfy markdown linters.

📝 Proposed fix
-```
+```text
 score = urgency + (blocked_tasks_count × 1.5)
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

Verify each finding against the current code and only fix it if needed.

In @AGENTS.md around lines 244 - 246, The fenced code block showing the formula
"score = urgency + (blocked_tasks_count × 1.5)" is missing a language specifier;
update the markdown fence to include a language such as "text" or "plaintext" so
linters recognize it as plain text (referencing the formula symbols score,
urgency, and blocked_tasks_count), e.g., change totext and keep the
formula unchanged.


</details>

</blockquote></details>
<details>
<summary>src/instant_messaging/L2PSMessagingPeer.ts (5)</summary><blockquote>

`180-183`: **Use `_` for the unused loop variable.**

The `key` variable is declared but never used. Use `_` to indicate intentional discard.

<details>
<summary>🧹 Proposed fix</summary>

```diff
-        for (const [key, pending] of this.pendingResponses) {
+        for (const [_, pending] of this.pendingResponses) {
             clearTimeout(pending.timer)
             pending.reject(new Error("Disconnected"))
         }
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 180 - 183, The
for-loop over this.pendingResponses introduces an unused variable named key;
update the loop to discard the first tuple element (use _ or an empty slot) so
the intent is clear and linting warnings are avoided — e.g., change the
iteration in L2PSMessagingPeer that currently reads for (const [key, pending] of
this.pendingResponses) to use a discard for the key (for (const [, pending] ...)
or for (const [_ , pending] ...)) while leaving the clearTimeout(pending.timer)
and pending.reject(new Error("Disconnected")) logic unchanged.
```

</details>

---

`28-28`: **Remove unused import `StoredMessage`.**

Static analysis indicates `StoredMessage` is imported but never used in this file.

<details>
<summary>🧹 Proposed fix</summary>

```diff
     ErrorCode,
-    StoredMessage,
 } from "./l2ps_types"
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` at line 28, The import list in
L2PSMessagingPeer.ts includes an unused symbol StoredMessage; remove
StoredMessage from the import statement (where StoredMessage is referenced) so
the file only imports actually used symbols, e.g., update the import that
currently lists StoredMessage to exclude it.
```

</details>

---

`479-486`: **Use optional chaining for cleaner code.**

The `meta` check can be simplified with optional chaining.

<details>
<summary>♻️ Proposed fix</summary>

```diff
         if (frame.requestId && this.pendingResponses.has(frame.requestId)) {
             const pending = this.pendingResponses.get(frame.requestId)!
-            const meta = pending._meta
-            if (meta && meta.types.includes(frame.type)) {
+            if (pending._meta?.types.includes(frame.type)) {
                 clearTimeout(pending.timer)
                 this.pendingResponses.delete(frame.requestId)
                 pending.resolve(frame.payload)
                 return
             }
         }
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 479 - 486, Replace
the explicit null check on pending._meta with optional chaining to simplify the
condition in L2PSMessagingPeer: instead of checking meta and then
meta.types.includes(frame.type), use pending._meta?.types.includes(frame.type)
(or meta?.types.includes(frame.type) if you keep the local variable). Keep the
rest of the logic (clearTimeout, delete from pendingResponses, resolve) intact.
```

</details>

---

`82-107`: **Consider marking non-reassigned members as `readonly`.**

Static analysis identifies several members that are never reassigned. Adding `readonly` improves type safety by preventing accidental reassignment.

<details>
<summary>♻️ Proposed fix</summary>

```diff
 export class L2PSMessagingPeer {
     private ws: WebSocket | null = null
-    private config: L2PSMessagingConfig
+    private readonly config: L2PSMessagingConfig

     // Event handlers
-    private messageHandlers: Set<L2PSMessageHandler> = new Set()
-    private errorHandlers: Set<L2PSErrorHandler> = new Set()
-    private peerJoinedHandlers: Set<L2PSPeerHandler> = new Set()
-    private peerLeftHandlers: Set<L2PSPeerHandler> = new Set()
-    private connectionStateHandlers: Set<L2PSConnectionStateHandler> = new Set()
+    private readonly messageHandlers: Set<L2PSMessageHandler> = new Set()
+    private readonly errorHandlers: Set<L2PSErrorHandler> = new Set()
+    private readonly peerJoinedHandlers: Set<L2PSPeerHandler> = new Set()
+    private readonly peerLeftHandlers: Set<L2PSPeerHandler> = new Set()
+    private readonly connectionStateHandlers: Set<L2PSConnectionStateHandler> = new Set()

     // Pending request-response waiters
-    private pendingResponses: Map<...> = new Map()
+    private readonly pendingResponses: Map<...> = new Map()

     // State
-    private messageQueue: OutgoingFrame[] = []
+    private readonly messageQueue: OutgoingFrame[] = []

     // Reconnection
-    private maxReconnectAttempts = 10
-    private baseReconnectDelay = 1000
+    private readonly maxReconnectAttempts = 10
+    private readonly baseReconnectDelay = 1000
```
</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 82 - 107, Several
instance fields are never reassigned and should be marked readonly to improve
type safety: add the readonly modifier to messageHandlers, errorHandlers,
peerJoinedHandlers, peerLeftHandlers, connectionStateHandlers, pendingResponses,
onlinePeers, messageQueue, maxReconnectAttempts and baseReconnectDelay (leave
mutable state fields like _isConnected/_isRegistered and reconnectAttempts
unchanged). Locate these members in L2PSMessagingPeer (the declarations shown
for messageHandlers, errorHandlers, peerJoinedHandlers, peerLeftHandlers,
connectionStateHandlers, pendingResponses, onlinePeers, messageQueue,
maxReconnectAttempts, baseReconnectDelay) and prefix them with readonly so they
cannot be reassigned later.
```

</details>

---

`316-334`: **Add JSDoc comments to public event handler methods.**

Per coding guidelines, new methods should have JSDoc documentation. These public API methods would benefit from brief descriptions for IDE support and discoverability.

<details>
<summary>📝 Example JSDoc additions</summary>

```typescript
/** Register a handler for incoming messages */
onMessage(handler: L2PSMessageHandler): void { ... }

/** Register a handler for protocol errors */
onError(handler: L2PSErrorHandler): void { ... }

/** Register a handler for peer join events */
onPeerJoined(handler: L2PSPeerHandler): void { ... }

/** Register a handler for peer leave events */
onPeerLeft(handler: L2PSPeerHandler): void { ... }

/** Register a handler for connection state changes */
onConnectionStateChange(handler: L2PSConnectionStateHandler): void { ... }
```
</details>

As per coding guidelines: "Use JSDoc format for all new methods and functions".

<details>
<summary>🤖 Prompt for AI Agents</summary>

```
Verify each finding against the current code and only fix it if needed.

In `@src/instant_messaging/L2PSMessagingPeer.ts` around lines 316 - 334, Add JSDoc
comments to the public event registration methods to follow coding guidelines:
for each of onMessage, onError, onPeerJoined, onPeerLeft, and
onConnectionStateChange add a brief JSDoc summary sentence, an `@param` tag
documenting the handler argument and its type (e.g., L2PSMessageHandler,
L2PSErrorHandler, L2PSPeerHandler, L2PSConnectionStateHandler), and an `@returns`
tag indicating void; place the comment directly above each method declaration so
IDEs surface the descriptions and parameter types.
```

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🤖 Prompt for all review comments with AI agents</summary>

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @src/instant_messaging/L2PSMessagingPeer.ts:

  • Around line 377-381: In the catch block that notifies this.errorHandlers about
    a re-registration failure inside L2PSMessagingPeer, replace the current
    String(err) stringification with the same robust pattern used at line 411 (e.g.,
    use err instanceof Error ? err.message : JSON.stringify(err,
    Object.getOwnPropertyNames(err)) or similar) so non-Error objects don't
    stringify to "[object Object]"; update the call to h(...) in the re-registration
    catch to include that improved error string.

Nitpick comments:
In @AGENTS.md:

  • Around line 244-246: The fenced code block showing the formula "score =
    urgency + (blocked_tasks_count × 1.5)" is missing a language specifier; update
    the markdown fence to include a language such as "text" or "plaintext" so
    linters recognize it as plain text (referencing the formula symbols score,
    urgency, and blocked_tasks_count), e.g., change totext and keep the
    formula unchanged.

In @src/instant_messaging/L2PSMessagingPeer.ts:

  • Around line 180-183: The for-loop over this.pendingResponses introduces an
    unused variable named key; update the loop to discard the first tuple element
    (use _ or an empty slot) so the intent is clear and linting warnings are avoided
    — e.g., change the iteration in L2PSMessagingPeer that currently reads for
    (const [key, pending] of this.pendingResponses) to use a discard for the key
    (for (const [, pending] ...) or for (const [_ , pending] ...)) while leaving the
    clearTimeout(pending.timer) and pending.reject(new Error("Disconnected")) logic
    unchanged.
  • Line 28: The import list in L2PSMessagingPeer.ts includes an unused symbol
    StoredMessage; remove StoredMessage from the import statement (where
    StoredMessage is referenced) so the file only imports actually used symbols,
    e.g., update the import that currently lists StoredMessage to exclude it.
  • Around line 479-486: Replace the explicit null check on pending._meta with
    optional chaining to simplify the condition in L2PSMessagingPeer: instead of
    checking meta and then meta.types.includes(frame.type), use
    pending._meta?.types.includes(frame.type) (or meta?.types.includes(frame.type)
    if you keep the local variable). Keep the rest of the logic (clearTimeout,
    delete from pendingResponses, resolve) intact.
  • Around line 82-107: Several instance fields are never reassigned and should be
    marked readonly to improve type safety: add the readonly modifier to
    messageHandlers, errorHandlers, peerJoinedHandlers, peerLeftHandlers,
    connectionStateHandlers, pendingResponses, onlinePeers, messageQueue,
    maxReconnectAttempts and baseReconnectDelay (leave mutable state fields like
    _isConnected/_isRegistered and reconnectAttempts unchanged). Locate these
    members in L2PSMessagingPeer (the declarations shown for messageHandlers,
    errorHandlers, peerJoinedHandlers, peerLeftHandlers, connectionStateHandlers,
    pendingResponses, onlinePeers, messageQueue, maxReconnectAttempts,
    baseReconnectDelay) and prefix them with readonly so they cannot be reassigned
    later.
  • Around line 316-334: Add JSDoc comments to the public event registration
    methods to follow coding guidelines: for each of onMessage, onError,
    onPeerJoined, onPeerLeft, and onConnectionStateChange add a brief JSDoc summary
    sentence, an @param tag documenting the handler argument and its type (e.g.,
    L2PSMessageHandler, L2PSErrorHandler, L2PSPeerHandler,
    L2PSConnectionStateHandler), and an @returns tag indicating void; place the
    comment directly above each method declaration so IDEs surface the descriptions
    and parameter types.

</details>

<details>
<summary>🪄 Autofix (Beta)</summary>

Fix all unresolved CodeRabbit comments on this PR:

- [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended)
- [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes

</details>

---

<details>
<summary>ℹ️ Review info</summary>

<details>
<summary>⚙️ Run configuration</summary>

**Configuration used**: Organization UI

**Review profile**: CHILL

**Plan**: Pro

**Run ID**: `121db510-6f49-4afe-8086-15bd20265f0d`

</details>

<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between a9ac32124145c8ef8dafb66b0997307ff48d7e83 and ec99a6650c543b1f004084d2464d8c846794c5a3.

</details>

<details>
<summary>⛔ Files ignored due to path filters (1)</summary>

* `.mycelium/mycelium.db` is excluded by `!**/*.db`

</details>

<details>
<summary>📒 Files selected for processing (4)</summary>

* `.mycelium/.gitignore`
* `.serena/project.yml`
* `AGENTS.md`
* `src/instant_messaging/L2PSMessagingPeer.ts`

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>

* .mycelium/.gitignore

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@sonarqubecloud
Copy link
Copy Markdown

@tcsenpai tcsenpai merged commit 21fa5b0 into main Mar 26, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants